Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"Questionable Pull": Fixed BigInt bug & added BigFloat #532

Merged
merged 6 commits into from
Mar 7, 2012
Merged

"Questionable Pull": Fixed BigInt bug & added BigFloat #532

merged 6 commits into from
Mar 7, 2012

Conversation

choffstein
Copy link

I fixed a bug in the wrapper library and added some code to enable a BigFloat.

Unfortunately, my BigFloat tests don't pass because I haven't quite figured out the type-conversions. If someone knowledgable can take a look at jl/bigfloat.jl at the conversions, they should be fix it pretty quickly to get the tests passing.

@choffstein
Copy link
Author

I probably should have split this into two pull requests -- the first to fix BigInt, and the second to add BigFloat. If you only want to do the former, you can change gmp_wrapper.c _jl_mpz_printf to the following:

extern char*  _jl_mpz_printf(mpz_t* rop) {
    char* pp;
    gmp_asprintf(&pp, "%Zd", *rop);
    return pp;
}

@aviks
Copy link
Member

aviks commented Mar 7, 2012

I've managed to get the converts changed enough to give us correct arithmetic

julia> a=BigFloat("12.34567890121")
12.34567890121

julia> a+0.00000000001
12.3456789012199999999

However, some more work is needed on the comparisons. In general, == will not work for comparing floats on the tests, we'll need to use abs(..) < eps. But even beyond that, I am having problems with the comparisons, they seem inconsistent. I'll take a further look tomorrow.

Another minor comment: do we want to implicitly convert Unsigned values? or just singed ones?

@aviks
Copy link
Member

aviks commented Mar 7, 2012

@choffstein : The following is the diff that I have to your bigfloat at the moment.

bash-3.2$ git diff jl/bigfloat.jl 
diff --git a/jl/bigfloat.jl b/jl/bigfloat.jl
index 2f6cb57..bbbc5bf 100644
--- a/jl/bigfloat.jl
+++ b/jl/bigfloat.jl
@@ -13,7 +13,7 @@ type BigFloat <: Float
                b
        end

-       function BigFloat(x::Float) 
+       function BigFloat(x::Float64) 
                z = _jl_BigFloat_init()
                ccall(dlsym(_jl_libgmp_wrapper, :_jl_mpf_set_d), Void, (Ptr{Void}, Float), z, x)
                b = new(z)
@@ -52,18 +52,24 @@ type BigFloat <: Float
        end
 end

-convert(::Type{BigFloat}, x::Int8)   = BigFloat(x)
-convert(::Type{BigFloat}, x::Int16)  = BigFloat(x)
-convert(::Type{BigFloat}, x::Int32)  = BigFloat(x)
-convert(::Type{BigFloat}, x::Int64)  = BigFloat(x)
-convert(::Type{BigFloat}, x::Uint8)  = BigFloat(x)
-convert(::Type{BigFloat}, x::Uint16) = BigFloat(x)
-convert(::Type{BigFloat}, x::Uint32) = BigFloat(x)
-convert(::Type{BigFloat}, x::Uint64) = BigFloat(x)
-
-convert(::Type{BigFloat}, x::Float) = BigFloat(x)
-convert(::Type{BigFloat}, x::Float32) = BigFloat(x)
+convert(::Type{BigFloat}, x::Int8)   = BigFloat(int(x))
+convert(::Type{BigFloat}, x::Int16)  = BigFloat(int(x))
+convert(::Type{BigFloat}, x::Int)  = BigFloat(x)
+macro define_bigint_convert ()
+       if WORD_SIZE == 64
+               :(convert(::Type{BigFloat}, x::Int32) = BigInt(int(x)))
+               :(convert(::Type{BigFloat}, x::Uint32) = BigFloat(int(x)))
+
+       else
+               :(convert(::Type{BigFloat}, x::Int64) = BigInt(string(x)))
+               :(convert(::Type{BigFloat}, x::Uint64) = BigFloat(int(x)))
+       end
+end
+@define_bigint_convert
+convert(::Type{BigFloat}, x::Uint8)  = BigFloat(int(x))
+convert(::Type{BigFloat}, x::Uint16) = BigFloat(int(x))
 convert(::Type{BigFloat}, x::Float64) = BigFloat(x)
+convert(::Type{BigFloat}, x::Float32) = BigFloat(float64(x))

 promote_rule(::Type{BigFloat}, ::Type{Float32}) = BigFloat
 promote_rule(::Type{BigFloat}, ::Type{Float64}) = BigFloat

ViralBShah added a commit that referenced this pull request Mar 7, 2012
@ViralBShah
Merge pull request #532 from choffstein/master
5b8a93b 
"Questionable Pull": Fixed BigInt bug & added BigFloat. Merging the whole thing for now, since it won't affect anything else.
@ViralBShah ViralBShah merged commit 5b8a93b into JuliaLang:master Mar 7, 2012
@nolta
Copy link
Member

nolta commented Mar 7, 2012

Hmm, i don't think type-conversion is the problem. The tests fail even if the float literals are replaced with BigFloats.

Are you sure the tests shouldn't fail? I came across this in the manual:

The mantissa in stored in binary, as might be imagined from the fact precisions are expressed in bits. One consequence of this is that decimal fractions like 0.1 cannot be represented exactly. The same is true of plain IEEE double floats. This makes both highly unsuitable for calculations involving money or other values that should be exact decimal fractions. (Suitably scaled integers, or perhaps rationals, are better choices.)

http://gmplib.org/manual/Floating_002dpoint-Functions.html

@aviks
Copy link
Member

aviks commented Mar 7, 2012

Yes, of course, tests like (a + .00000001 == b) will never work.

However, I would expected, for example, (a + .00000001 -b < .00000001 ) to work. Visually, given the rounding errors, that should work.

Even more simply, a > 0.0 does not work, but a > BigFloat(0) does .

Note that you'll need to apply my diff above, otherwise the addition itself does not work correctly.

@choffstein
Copy link
Author

Yeah, momentary lapse. I was thinking that they were infinite precision structures -- but they aren't. Thanks for helping to clean it up.

@aviks
Copy link
Member

aviks commented Mar 7, 2012

There is an assert_approx_eq macro in runtest.jl now that seems to compare to an difference of 1.0e-6

Keno pushed a commit that referenced this pull request Oct 9, 2023
@simeonschaub
update builtins, cleanup pre-1.6 code paths (#532)
f74effe 
This makes `src/builtins.jl` in sync with `bin/generate_builtins.jl`
again. A lot of this code was also out-of-date, now that we only support
1.6, so this includes some cleanup.
Keno pushed a commit that referenced this pull request Oct 9, 2023
@timholy
Re-impose "no double lookup" for invoke (#540)
53ca034 
* Re-impose "no double lookup" for invoke

This was added in #442 and inadvertently removed in #532.
However, the test in #442 was not stringent enough; the test added here
comes from #535.

Fixes #535
Fixes timholy/Revise.jl#695
LilithHafner pushed a commit that referenced this pull request May 25, 2024
@DilumAluthgeBot @DilumAluthge
🤖 [master] Bump the SparseArrays stdlib from a09f90b to 82b385f (#54576)
951cd19 
Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: a09f90b
New commit: 82b385f
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @LilithHafner
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@a09f90b...82b385f

```
$ git log --oneline a09f90b..82b385f
82b385f Make tests expect FieldError exception once that exception is available in Base
9d4397f Sparse versions of repeat for matrices and vectors (#532)
```

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
DilumAluthge added a commit that referenced this pull request Jun 3, 2024
@DilumAluthgeBot @DilumAluthge
🤖 [master] Bump the SparseArrays stdlib from a09f90b to 82b385f (#54576)
07ec8a7 
Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: a09f90b
New commit: 82b385f
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @LilithHafner
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@a09f90b...82b385f

```
$ git log --oneline a09f90b..82b385f
82b385f Make tests expect FieldError exception once that exception is available in Base
9d4397f Sparse versions of repeat for matrices and vectors (#532)
```

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Jul 12, 2024
@DilumAluthgeBot @DilumAluthge @lazarusA
🤖 [master] Bump the SparseArrays stdlib from a09f90b to 82b385f (Juli…
24d824c 
…aLang#54576)

Stdlib: SparseArrays
URL: https://github.com/JuliaSparse/SparseArrays.jl.git
Stdlib branch: main
Julia branch: master
Old commit: a09f90b
New commit: 82b385f
Julia version: 1.12.0-DEV
SparseArrays version: 1.12.0
Bump invoked by: @LilithHafner
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaSparse/SparseArrays.jl@a09f90b...82b385f

```
$ git log --oneline a09f90b..82b385f
82b385f Make tests expect FieldError exception once that exception is available in Base
9d4397f Sparse versions of repeat for matrices and vectors (JuliaLang#532)
```

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@choffstein @aviks @nolta @ViralBShah